Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add integration tests against Postgres.js #479

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

hauleth
Copy link
Contributor

@hauleth hauleth commented Nov 12, 2024

What kind of change does this PR introduce?

Add integration tests against Postgres.js Node library.

@hauleth hauleth requested a review from a team as a code owner November 12, 2024 14:30
@hauleth hauleth marked this pull request as draft November 12, 2024 14:30
@hauleth hauleth force-pushed the test/add-postgresjs-integration-tests branch from f94e985 to 1ed6868 Compare November 12, 2024 14:30
@hauleth hauleth changed the title test: move HandlerHelpers tests to their module test: add integration tests against Postgres.js Nov 12, 2024
@hauleth hauleth force-pushed the test/add-postgresjs-integration-tests branch 3 times, most recently from 2279197 to 5903fbf Compare November 13, 2024 21:04
@hauleth hauleth marked this pull request as ready for review November 13, 2024 21:04
@hauleth hauleth force-pushed the test/add-postgresjs-integration-tests branch 11 times, most recently from 1826e32 to f6d8907 Compare November 14, 2024 16:46
initialScript = ''
${builtins.readFile ./dev/postgres/00-setup.sql}

CREATE USER postgres SUPERUSER PASSWORD 'postgres';
'';
listen_addresses = "127.0.0.1";
port = 6432;
settings = {
max_prepared_transactions = 262143;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is maximal possible value, in tests I have set it to 2000 as it doesn't matter really as long as it is greater than 0 I think.

Comment on lines +141 to +142
otp-version: '25.3.2.7'
elixir-version: '1.14.5'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering if we want to put these versions in the env vars

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be useful, but as a separate PR. There I simply copied what we have in other steps.

Comment on lines +1 to +2
1 2 3
4 5 6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does it do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea, it is copied as is from postgres.js test folder. I haven't really checked if that is used anywhere in test suite.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's try to run it without this file :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have found that it is used in one test:

t('Copy from file', async() => {
  await sql`create table test (x int, y int, z int)`
  await new Promise(async r => fs
    .createReadStream(rel('copy.csv'))
    .pipe(await sql`copy test from stdin`.writable())
    .on('finish', r)
  )

  return [
    JSON.stringify(await sql`select * from test`),
    '[{"x":1,"y":2,"z":3},{"x":4,"y":5,"z":6}]',
    await sql`drop table test`
  ]
})

@hauleth hauleth force-pushed the test/add-postgresjs-integration-tests branch from f6d8907 to 458fc9d Compare November 15, 2024 11:09
@hauleth hauleth merged commit 32c0086 into v2 Nov 15, 2024
6 checks passed
@hauleth hauleth deleted the test/add-postgresjs-integration-tests branch November 15, 2024 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants